Arm backend: Make per-channel quantization default#11873
Merged
Sebastian-Larsson merged 4 commits intoJul 15, 2025
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/11873
Note: Links to docs will display an error until the docs builds have been completed. ❌ 2 New FailuresAs of commit cfaa79f with merge base 2bd96df ( NEW FAILURES - The following jobs have failed:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
facebook-github-bot
added
the
CLA Signed
Collaborator
Author
|
@pytorchbot label ciflow/trunk |
Collaborator
Author
|
@pytorchbot label "partner: arm" |
pytorch-bot
Bot
added
the
partner: arm
Collaborator
Author
|
@pytorchbot label "topic: not user facing" |
zingo
added
release notes: arm
Contributor
|
@martinlsm no need to add |
digantdesai
reviewed
Jun 26, 2025
digantdesai
left a comment
digantdesai
left a comment
Contributor
There was a problem hiding this comment.
I assume rationale is accuracy but can you please add more details? And linear and conv weights only right? perf impact? I also didn't see any ATOL/RTOL change in this diff.
| "# Create and configure quantizer to use a symmetric quantization config globally on all nodes\n", | ||
| "quantizer = EthosUQuantizer(compile_spec)\n", | ||
| "operator_config = get_symmetric_quantization_config(is_per_channel=False)\n", | ||
| "operator_config = get_symmetric_quantization_config(is_per_channel=True)\n", |
Contributor
There was a problem hiding this comment.
Nit
Suggested change
| "operator_config = get_symmetric_quantization_config(is_per_channel=True)\n", | |
| "operator_config = get_symmetric_quantization_config()\n", |
Collaborator
Author
There was a problem hiding this comment.
@digantdesai I have resolved your code comment and answered all your questions in the updated commit message.
martinlsm
force-pushed
the
marlin-per-channel-quant
branch
from
8175bb3 to
c3567b4
Compare
digantdesai
approved these changes
Jul 2, 2025
digantdesai
left a comment
digantdesai
left a comment
Contributor
There was a problem hiding this comment.
I am glad to see we didn't have to mess with the backend code at all when quantizing the weights differently. Thanks @martinlsm.
Collaborator
@digantdesai we did mess with the backend code but in a separate PR #11752. |
martinlsm
force-pushed
the
marlin-per-channel-quant
branch
from
fb40b0a to
4dd805c
Compare
Support for per-channel quantization was recently added to the Arm backend. This patch changes the default setting to use per-channel quantization for weights in convolutional and linear layers, instead of per-tensor quantization, which was the previous default. The reason for this change is that per-channel quantization offers better numerical accuracy for models containing convolutional and/or fully connected layers. Unless there is an explicit limitation in the use case that prevents the use of per-channel quantization, it is generally preferred. The option to set quantization granularity can still be manually set using `get_symmetric_quantization_config(is_per_channel=False)`. This patch only changes the default. Unit and model tests are affected by this change. Error tolerances for those tests have not been changed, as model outputs are compared against a reference that uses the exact same quantization strategy. That is, if a model output is altered by this patch, the reference it is compared against would also be altered accordingly. To verify the impact of this change in terms of top-1 and top-5 accuracy, a manual test was run on MobileNetV2. The results show a noticeable improvement: - Per-tensor quantization Top-1 / Top-5 accuracy: 66.45% / 87.50% - Per-channel quantization Top-1 / Top-5 accuracy: 70.85% / 89.50% Change-Id: I35d5c62741c7f93b916560874689245db96a588b Signed-off-by: Martin Lindström <Martin.Lindstroem@arm.com>
Previously we were just a few minutes off the 90 minute timeout. With per-channel quantizaiton enabled by defualt it seems that we exceed that limit consistently. This patch increases the timeout to 120 minutes. Change-Id: I20f3fb369329dd51e95ffec667617afe93c50aa3 Signed-off-by: Oscar Andersson <oscar.andersson@arm.com>
oscarandersson8218
force-pushed
the
marlin-per-channel-quant
branch
from
4dd805c to
07c2ba7
Compare
Contributor
|
LOL That's what I thought :D Are we ready to merge this one then? |
Sebastian-Larsson
approved these changes
Jul 15, 2025
Sebastian-Larsson
left a comment
Sebastian-Larsson
left a comment
Collaborator
There was a problem hiding this comment.
Unrelated CI failures
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
8 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Support for per-channel quantization was recently added to the Arm
backend. This patch changes the default setting to use per-channel
quantization for weights in convolutional and linear layers, instead of
per-tensor quantization, which was the previous default.
The reason for this change is that per-channel quantization offers
better numerical accuracy for models containing convolutional and/or
fully connected layers. Unless there is an explicit limitation in the
use case that prevents the use of per-channel quantization, it is
generally preferred.
The option to set quantization granularity can still be manually set
using
get_symmetric_quantization_config(is_per_channel=False). Thispatch only changes the default.
Unit and model tests are affected by this change. Error tolerances for
those tests have not been changed, as model outputs are compared against
a reference that uses the exact same quantization strategy. That is, if
a model output is altered by this patch, the reference it is compared
against would also be altered accordingly.
To verify the impact of this change in terms of top-1 and top-5
accuracy, a manual test was run on MobileNetV2. The results show a
noticeable improvement:
cc @digantdesai @freddan80 @per @zingo @oscarandersson8218